Skip to content

Conversation

@mu001999
Copy link
Contributor

@mu001999 mu001999 commented Nov 7, 2025

This PR lints unused associated types, like associated consts and fns.

Different from assoc consts and fns, associated types correspond to QPath::TypeRelative, and Res::Err will be obtained in HIR. Therefore, it is necessary to traverse rustc_middle::ty::Ty to help propagate liveness to the corresponding associated type.

Fixes #127673
Fixes #110332

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 7, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 7, 2025

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
Copy link
Member

BoxyUwU commented Nov 7, 2025

does this handle assoc types used only from item signatures and not from within bodies?

@mu001999

This comment was marked as outdated.

@rustbot
Copy link
Collaborator

rustbot commented Nov 8, 2025

Some changes occurred in tests/ui/sanitizer

cc @rcvalle

@rustbot rustbot added the PG-exploit-mitigations Project group: Exploit mitigations label Nov 8, 2025
@fmease
Copy link
Member

fmease commented Nov 8, 2025

does this handle assoc types used only from item signatures and not from within bodies?

This also handles assoc types used from within bodies, by checking type of expr,

She meant the exact opposite. Assoc tys only used in non-bodies (aka "item signatures", i.e., places where we don't infer types, i.e., not fn or const bodies, but e.g., inside ADTs, type aliases, impl headers, etc.).

@mu001999
Copy link
Contributor Author

mu001999 commented Nov 8, 2025

@fmease Hi, I add some tests for such cases but I'm not sure they are right or good enough. Could you have a look or give some examples? Thanks!

@fmease
Copy link
Member

fmease commented Nov 8, 2025

Personally speaking, I was thinking of

trait Trait { type Ty; }

impl Trait for () { type Ty = (); }

pub struct Wrap(<() as Trait>::Ty); // <- use of QPath::Resolved `Ty` in a non-body only

impl Wrap {
    pub fn live(self) { _ = self.0; }
}

or

trait Trait { type Ty; }

impl Trait for () { type Ty = (); }

pub struct Wrap(Inner<()>);
struct Inner<T: Trait>(T::Ty); // <- use of QPath::TypeRelative `Ty` in a non-body only

impl Wrap {
    pub fn live(self) { _ = self.0; }
}

I'm sure my examples can be simplified. Also, I haven't really thought much about all of this yet as I'm just drive-by reviewing, so feel free to correct me anywhere :)

@mu001999 mu001999 force-pushed the dead-code/assoc-ty branch 2 times, most recently from 6a4d4a2 to 4f6cdd6 Compare November 8, 2025 00:58
@mu001999
Copy link
Contributor Author

mu001999 commented Nov 8, 2025

@fmease Added and passed.

But current implementation for struct fields is special. Simply speaking, types of fields in used struct will be marked live directly whether the field self is used or not.

The detail is described in https://github.com/rust-lang/rust/pull/148654/files#diff-719b4f36bbbf3b3bf48683429408f5d27e5633f70c1144a93554bd536d5bf655R753-R764

@mu001999
Copy link
Contributor Author

mu001999 commented Nov 8, 2025

does this handle assoc types used only from item signatures and not from within bodies?

Yes, such assoc types are handled by visiting predicates of the source items.

@mu001999
Copy link
Contributor Author

mu001999 commented Nov 9, 2025

@fmease I test and find this also fixes #110332

@fee1-dead
Copy link
Member

I am not confident enough that this code works well enough, I am worried about the false positives. E.g., what about projected types on impls:

pub struct A;

trait B {
    type Assoc;
}

impl B for A {
    type Assoc = A;
}

trait C {}

impl C for <A as B>::Assoc {}

What about types used in free-consts, e.g. const X: <A as B>::Assoc = A;?

This either needs to have solid coverage or land as an allowed-by-default lint for now.

@rustbot
Copy link
Collaborator

rustbot commented Nov 12, 2025

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@mu001999
Copy link
Contributor Author

@fee1-dead Added test cases for projectd types on impl and free const.

@fee1-dead
Copy link
Member

Sorry, don't have enough time right now

@rustbot reroll

@rustbot rustbot assigned nnethercote and unassigned fee1-dead Nov 14, 2025
@fmease
Copy link
Member

fmease commented Nov 14, 2025

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Nov 14, 2025
@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 14, 2025
@rust-bors
Copy link

rust-bors bot commented Nov 14, 2025

☀️ Try build successful (CI)
Build commit: 016d8ef (016d8ef60defc90251b80016da593e2c44d26cbf, parent: 42f4793e5a514858221b07ac379029d90353913e)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (016d8ef): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.3% [0.1%, 1.0%] 97
Regressions ❌
(secondary)
0.4% [0.0%, 1.1%] 72
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.3% [0.1%, 1.0%] 97

Max RSS (memory usage)

Results (primary 2.8%, secondary -3.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
4.8% [2.5%, 7.2%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.2% [-1.2%, -1.2%] 1
Improvements ✅
(secondary)
-3.4% [-3.4%, -3.4%] 1
All ❌✅ (primary) 2.8% [-1.2%, 7.2%] 3

Cycles

Results (primary 0.2%, secondary 2.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.5% [2.5%, 2.5%] 1
Regressions ❌
(secondary)
2.5% [2.5%, 2.5%] 1
Improvements ✅
(primary)
-2.2% [-2.2%, -2.2%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [-2.2%, 2.5%] 2

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 473.087s -> 474.173s (0.23%)
Artifact size: 388.70 MiB -> 388.71 MiB (0.00%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Nov 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf-regression Performance regression. PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lint unused assoc types although the trait is used dead_code: Inherent associated types are unconditionally considered dead

8 participants